release: 0.4.27.1 — backport Slack files_upload_v2 confirmation fix#120
release: 0.4.27.1 — backport Slack files_upload_v2 confirmation fix#120patrick-chinchill wants to merge 2 commits into
Conversation
SlackAdapter._upload_files already returns the list of Slack-confirmed file IDs, but post_message discarded the return and ThreadImpl._create_sent_message hardcoded raw=None, so the confirmation never reached SentMessage.raw. Slack was the only file-capable adapter to drop upload confirmation; discord/telegram upload inline and expose the platform response naturally. - post_message: capture _upload_files' return and augment RawMessage.raw with "uploaded_file_ids" on every return path that can carry files (file-only, card, table, text), via a new _augment_raw_with_uploads helper. None means no upload; empty list means Slack confirmed zero attachments. raw is augmented, not replaced — non-breaking. - thread.py: _create_sent_message accepts and propagates a raw parameter; the post() call site passes raw_msg.raw through. Platform-agnostic, so discord/telegram real platform responses now flow through too. Tests: 3 in test_slack_api.py (file-only, text+files, text-only no-augment) and 1 end-to-end in test_thread_faithful.py. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Post-merge review of PR #117 caught that _MessageHistoryCache.append in chat.py did not null out raw before persisting (unlike its sibling MessageHistoryCache.append in message_history.py, which already does). Without this, the platform raw payload (Slack team_id/user_id, Discord guild IDs, etc.) that #117 now correctly populates on SentMessage.raw would persist to Redis/Postgres-backed state on every reply, inflating storage size and PII surface. Mirrors the null-out in message_history.py:62.
|
Warning Review limit reached
More reviews will be available in 56 minutes and 49 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (7)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request bumps the SDK version to 0.4.27.1 and introduces a fix to surface Slack file upload confirmations (uploaded_file_ids) on SentMessage.raw. It also updates _MessageHistoryCache to null out the raw payload before persisting messages to history, preventing storage bloat and PII exposure. Feedback on this PR suggests explicitly documenting the SDK's versioning scheme to clarify how local, Python-only patch versions are handled relative to upstream releases to avoid potential version number collisions.
| [project] | ||
| name = "chat-sdk" | ||
| version = "0.4.27" | ||
| version = "0.4.27.1" |
There was a problem hiding this comment.
When defining a versioning scheme that embeds an upstream version (such as 0.4.27.1 for a local patch on 0.4.27), there is a potential for collision if the upstream project releases a patch version (e.g., 4.27.1). If upstream patch releases are handled by bumping to the next minor version in this SDK, please ensure this behavior is explicitly documented (for example, in the README or a dedicated versioning document) to clarify that the fourth digit/local patch slot is strictly reserved for local, Python-only fixes.
References
- When defining a versioning scheme that embeds an upstream version, ensure that the scheme clearly differentiates between upstream patch releases and local (Python-only) fixes to avoid ambiguity and potential collisions in version numbers. If upstream patch releases are handled by bumping to the next minor version, explicitly document this to clarify that the local patch slot is reserved for local fixes.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e45069f686
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| files = extract_files(message) | ||
| if files: | ||
| await self._upload_files(files, channel, thread_ts or None) | ||
| uploaded_file_ids = await self._upload_files(files, channel, thread_ts or None) |
There was a problem hiding this comment.
When this new return value is used, successful real Slack uploads will usually surface uploaded_file_ids: []: the Python SDK's files_upload_v2 returns the files.completeUploadExternal payload with top-level entries like {"files": [{"id": ...}]}, but _upload_files still only collects IDs from a nested uploaded["files"] list. As a result, consumers gating on SentMessage.raw["uploaded_file_ids"] get a false “zero attachments confirmed” signal even though the files were delivered; the extraction needs to handle the top-level file objects before this value is exposed.
Useful? React with 👍 / 👎.
|
Closing — this is a tag-only maintenance release, not a main merge. The fix also lands on main forward via #117 (the null-out commit e45069f is being cherry-picked onto #117 so the 0.4.29 line gets the PII guard too). Branch chinchill-api pins |
CHANGELOG: - Drop false gchat file-delivery bullet (PR #112 never merged; upstream itself does not implement gchat outbound file delivery) - Fix Slack DM block-action citation to merged PR #137 (supersedes #133) - Re-point 0.4.27.1 vehicle from unmerged #120 to tag v0.4.27.1 + #117 - Drop numberless Twilio 'scaffolding PR' (only #142 exists) UPSTREAM_SYNC: - Reframe Google Chat outbound file delivery as parity (upstream also logs 'not yet supported' + media.upload TODO, index.ts:1282-1289) - Correct Teams cert-auth row: TS is not 'Supported' — config throws at startup in both SDKs (types.ts:31, config.ts:13); parity, issue #58 - Add Messenger get_user row: upstream has no getUser; raising stub is parity, Graph-API impl tracked as #132
…PR 4/4) (#146) * chore(release): cut 0.4.30 — Teams SDK migration + 4.30.0 parity (#93 PR 4/4) Final PR of the Teams SDK migration (issue #93). PRs 1-3 (inbound+auth, outbound, native streaming) are merged; this cuts the 0.4.30 release. - Version bump: pyproject 0.4.29 -> 0.4.30; UPSTREAM_PARITY "4.29.0" -> "4.30.0". - Fidelity re-pin chat@4.29.0 -> chat@4.30.0 in lint.yml + verify_test_fidelity.py (docstring, default parity fallback, clone hint). packages/chat/src is byte-for-byte identical between the two tags, so zero new test ports: strict fidelity stays 100% (732/732, 0 missing) against chat@4.30.0. Baseline regenerated (ts_parity -> chat@4.30.0; the recorded total_ts_tests literal 731 -> 732 corrects a stale count from the merged adapter waves, not the re-pin — the count is identical against both tags). - Docs: project-instructions version map + fidelity pin; README status line; CHANGELOG 0.4.30 entry (Twilio adapter, Telegram streaming, Slack subpaths, WhatsApp/Slack/gchat fixes, Teams #93 PRs 1-4); UPSTREAM_SYNC.md parity header + the Teams deferral row flipped to delivered. - Version-label normalization: malformed `adapter-teams@chat@4.30.0` and loose `adapter-teams@4.30.0` -> `@chat-adapter/teams@4.30.0` in adapter.py (5), bridge.py (1), UPSTREAM_SYNC.md (4). Comment/doc-only. Does NOT tag/publish — the release is a separate maintainer-gated step (live Teams 429 streaming check + PyPI authorization). * docs(release): clarify 0.4.30 core-parity wording + finish label normalization Review fast-follow for PR 4. The CHANGELOG now states the mapped core is content-identical *between the chat@4.29.0 and chat@4.30.0 upstream tags* (verified: thread.ts/types.ts/thread.test.ts and the full packages/chat/src tree are byte-identical) — the prior 'unchanged from 4.29.0' phrasing was accurate but misread as a claim about our code. Also sweeps the two remaining old-style `adapter-teams@chat@4.30.0` labels in the Teams test docstrings to the canonical `@chat-adapter/teams@4.30.0` npm tag, completing the normalization the PR's scope called for. No logic change. * docs(release): document 0.4.30 parity-audit wave + remaining exceptions CHANGELOG: add 'Pre-existing parity gaps closed (4.30 audit)' subsection covering the 7 ported fixes (PRs #147-#150) and a documented-exceptions note (Linear agent-sessions #151 deferred to 4.31 / #152; adapter-web and the GitHub/Linear native-client + message.subject halves stay Known Non-Parity). UPSTREAM_SYNC: - file-mapping table: add state-ioredis -> redis.py (IoRedisStateAdapter) and state-pg -> postgres.py rows (both ported + tested, were missing). - Known Non-Parity (platform gaps): add Linear agent-sessions row (#151) and an adapter-web row that splits the portable server-side WebAdapter (deferred) from the genuinely browser-only client subpaths, correcting the earlier over-broad 'browser-only; no Python runtime' note. - Correct the Google Chat file-uploads row: inbound attachment parsing is fully implemented (_create_attachment), so the gap is outbound file delivery only (post_message still logs 'not yet supported'). * docs(release): correct phantom-feature framing before PyPI cut CHANGELOG: - Drop false gchat file-delivery bullet (PR #112 never merged; upstream itself does not implement gchat outbound file delivery) - Fix Slack DM block-action citation to merged PR #137 (supersedes #133) - Re-point 0.4.27.1 vehicle from unmerged #120 to tag v0.4.27.1 + #117 - Drop numberless Twilio 'scaffolding PR' (only #142 exists) UPSTREAM_SYNC: - Reframe Google Chat outbound file delivery as parity (upstream also logs 'not yet supported' + media.upload TODO, index.ts:1282-1289) - Correct Teams cert-auth row: TS is not 'Supported' — config throws at startup in both SDKs (types.ts:31, config.ts:13); parity, issue #58 - Add Messenger get_user row: upstream has no getUser; raising stub is parity, Graph-API impl tracked as #132 * docs(changelog): clarify 0.4.30 audit-gap count (8 closed + 1 deferred = 9)
2 participants
Summary
Point release 0.4.27.1 — Python-only patch on the 0.4.27 line. Does NOT include the 0.4.29 alpha sync work currently in flight on main. Branched off
v0.4.27tag.Unblocks chinchill-api consumer that needs to gate UX on actual Slack file-delivery confirmation, ahead of the 0.4.29 release.
What's in 0.4.27.1
Slack
files_upload_v2confirmation surfaced throughpost()(backported from fix(slack): surface files_upload_v2 confirmation in SentMessage.raw #117).SlackAdapter._upload_filesalready computed the list of Slack-confirmed file IDs butpost_messagediscarded the return, andThreadImpl._create_sent_messagehardcodedraw=None. Now:post_messageaugmentsRawMessage.rawwith"uploaded_file_ids"on every file-carrying return path;_create_sent_messageaccepts and propagates the adapter'sraw.None= no upload; empty list = Slack confirmed zero. Augmentation is additive, not a replace.Storage-bloat / PII fix —
chat._MessageHistoryCache.appendnow nullsrawbefore persisting (mirror of the existing null-out inmessage_history.MessageHistoryCache.append). Without this, the platform payload that the above fix populates onSentMessage.raw(Slackteam_id/user_id, DiscordguildId, etc.) would persist on every reply to Redis/Postgres-backed state. Caught by post-merge code review of fix(slack): surface files_upload_v2 confirmation in SentMessage.raw #117.Why a point release vs waiting for 0.4.29
0.4.29 is still in mid-wave (alpha 1 just landed; chat/ai, Messenger, and other big pieces still in flight). chinchill-api can't wait. Branching off
v0.4.27for a focused patch gets the fix to production immediately without coupling to 4.29's risk surface.Release mechanics
v0.4.27tag (c43ea05)release/0.4.27.1main(for visibility / CI / review). Not intended for merge into main — main is already on0.4.29a1and merging would conflict on version + CHANGELOG. After approval, tagv0.4.27.1fromrelease/0.4.27.1directly; CHANGELOG entry will be forward-ported to main via a separate small PR.Validation
ruff check+ruff format --check: cleanaudit_test_quality.py: 0 hard failuresUpstream convergence
Filing the same fix upstream (vercel/chat#564) to restore parity in a future sync.
Generated by Claude Code
Generated by Claude Code